gh-112821: Fix rlcompleter failures on objects with descriptors#149577
gh-112821: Fix rlcompleter failures on objects with descriptors#149577mdboom merged 2 commits intopython:mainfrom
Conversation
| # Also, getattr(thisobject, word) will evaluate the | ||
| # property method, which is not desirable. | ||
|
|
||
| class_attr = getattr(type(thisobject), word, None) |
There was a problem hiding this comment.
This could also run arbitrary code because the descriptor protocol also gets run on access through the class. To be fully safe we should probably use something like inspect.getattr_static. Still, your change is a strict improvement so I'm OK with landing it.
There was a problem hiding this comment.
I experimented with this but it breaks another test create_expected_for_none and changes behavior elsewhere. Upstream behavior and this PR:
>>> None.__class__<TAB>
None.__class__()
Changing this line to inspect.getattr_static(type(thisobject), word, None):
>>> None.__class__<TAB>
None.__class__
I could argue this either way, but I think I will merge this as-is as it doesn't change any existing tested behavior.
|
Thanks @mdboom for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14, 3.15. |
|
GH-149656 is a backport of this pull request to the 3.15 branch. |
|
GH-149657 is a backport of this pull request to the 3.14 branch. |
|
GH-149658 is a backport of this pull request to the 3.13 branch. |
This issue came to light for me when it was discovered that autocompletion didn't work for a library I help maintain, written mainly in Cython. Cython implements
@property-annotated methods on itscdef classes as getset descriptors. Therlcompleterlogic doesn't exclude these (like it does regular Python properties) sinceisinstance(member, property)is False. It then goes on to call the getter, which in some cases (in our library at least) raises an exception. Any exception other than anAttributeErrorcauses the rlcompleter to short circuit completely and not offer any options.This is a more surgical (rlcompleter-only) solution than what was proposed in #112821. If adding ABCs is preferred, I could do that.
#112821 also mentioned:
I think this is handled here, but I also may not fully understand this specific issue.